Repository navigation
ASoC: tas2764, tas2770: do not log a deferred probe as an error - #166
chriskearney merged 2 commits into
Conversation
|
More evidence from the same 7.2.10-1-2-ARCH run.
The patch only changes how a deferral is logged. It does not change probe order. |
|
CHANGES at bf6c4fa. K review lane, raw review of the patch and the evidence. I did not build or boot this change. The PR has no checks yet, so the CI boot job the body asks for has not run. The two call-site edits are right in intent. Items 1 and 2 are small fixes, item 3 is a choice.
If
Evidence. The 36 lines, the six bound amplifiers and the 2 s tone are consistent with the change. The follow-up comment correctly says the deferral depends on probe timing: the 14-inch run printed four on the stock kernel and none on the candidate. The commit message sentence "The previous kernel, 7.1.12, printed none of them on the same machine" is true only for those boots. I would add "in the boots compared". Posted as a comment review. Merge is for the omacom maintainers. |
tas2764_parse_dt() defers when the reset GPIO, the shutdown GPIO or the SDZ supply is not ready. tas2770_parse_dt() defers when the shutdown GPIO or the SDZ supply is not ready. The probe functions then print "Failed to parse devicetree" through dev_err() on every retry. On a 16-inch MacBook Pro (M1 Max) running 7.2.10-1-2-ARCH this prints 36 error lines in one boot (six amplifiers, six probe attempts each), and all six amplifiers bind afterwards. 7.1.12 printed none of them on the same machine in the boots compared. Do not log -EPROBE_DEFER at this site. A deferral from the supply already carries its own reason from dev_err_probe() in parse_dt, and logging again with dev_err_probe() here would replace that reason in devices_deferred. Other errors are still logged as before. Signed-off-by: Joshua Warren <[email protected]>
bf6c4fa to
7f10359
Compare
|
Thanks. All three findings were right. Pushed
Still not built or booted. |
|
GREEN at 7f10359. K review lane, re-review of the updated patch. I did not build or boot it. The PR still has no checks, so the CI boot job has not run. My three findings are addressed:
The change is two three-line edits of the same shape. Non-blocking: when Posted as a comment review. Merge is for the omacom maintainers and needs the CI boot job. |
|
Status check from the author: the two Kernel CI runs for this branch are both Maintainer action needed: approve the Kernel CI workflow runs for this PR (Actions tab, "Approve and run"), then review. What is known: the change only keeps |
|
That probe-deferral fix is sitting in Kernel CI. I need you to approve the run so we can get the first CI build. |
The compile job exhausted its 30-minute budget in SHA checkout and history fetch before any compiler ran. Fetch the event merge ref, which carries both parents, and keep an explicit merge-base failure gate. Automatic team build gating remains unchanged. Signed-off-by: Joshua Warren <[email protected]> Generated-by: LLM
[Medium impact] The PR appears safe to merge; no actionable issues were found. SummaryThe drivers stop logging deferred probes as device-tree errors. Other errors and return values stay unchanged.
Intent noted: Reviews (1) · Last reviewed commit: "ci: fetch the named event ref before per..." · Reviewed by Greptile |
|
That CI job hit its time limit while fetching Git History. So the compiler never ran. Now, in a7bf189, CI fetches the named merge ref instead of raw SHAs. The merge-base check stays. The team auto-build rule is unchanged. The local Git test passes. |
The plan job read .github/VOUCHED.td at pull_request.base.sha, which stays at the base a pull request was opened against. Every pull request opened before the list existed found no list, so joshuaswarren's run on omacom#166 was approved and then planned checks only. Read the list from the target branch by name: only merged changes reach it, so a pull request still cannot vouch for itself. Signed-off-by: Marcelo Alcantara <[email protected]>
chriskearney
left a comment
There was a problem hiding this comment.
Reviewed a7bf189, including all four changed files. No concrete blocker found.
Both codec changes suppress only the outer -EPROBE_DEFER error message. Error returns, probe ordering, ordinary failure logs and the existing lower-level SDZ deferral reason are preserved. The CI change fetches the named event ref without moving the checkout and retains the final planned-base/head merge-base check, so missing history fails closed. The per-commit compiler still uses the planned identities.
Independent focused validation: the exact changed caller blocks passed 16 host cases per driver and caught four semantic mutants per driver; the authored workflow-fetch unittest and six additional exact-step local Git controls passed, including queue refs, deeper history and missing-commit/ref refusal. I read the retained report/results and independently reviewed the four-file diff and relevant workflow/planner path. These mocked host controls are not full driver or hardware execution.
Current PR compile/DT/host/result checks are green; package, KUnit and boot were skipped, which is not a pass. The author has not booted this exact patch on a Mac. Approval is for this bounded logging/CI change; normal combined-tree merge-queue CI remains required. The planner selects the full package/KUnit/boot tier for merge_group. No amplifier sound or physical-Mac boot qualification is claimed.
8e210de
What changed
tas2764_i2c_probe()andtas2770_i2c_probe()no longer logFailed to parse devicetreewhen*_parse_dt()returns-EPROBE_DEFER. Every other error is logged as before.Why
tas2764_parse_dt()defers when the reset GPIO, the shutdown GPIO or the SDZ supply is not ready.tas2770_parse_dt()defers when the shutdown GPIO or the SDZ supply is not ready. The probe printed the error line on every retry.I do not use
dev_err_probe()at this site. For the supply caseparse_dtalready recordsFailed to get SDZ supplyas the deferral reason, and a seconddev_err_probe()here would replace it indevices_deferred(found by w7K's review ofdrivers/base).Evidence
A 16-inch MacBook Pro (M1 Max,
apple,j316c), packagelinux-aurora7.2.10.aurora1-2 (release7.2.10-1-2-ARCH), one-shot boot from the stock Limine setup:/sys/bus/i2c/drivers/tas2764/lists 1-0038, 1-0039, 1-003a, 3-003b, 3-003c, 3-003d).snd-soc-macaudiologsSpeaker volumes unlocked. A 2 s tone played with exit 0.7.1.12-2-12.6-sep-ARCHon the same machine: 0 lines in the boots compared.devices_deferred, so which of the three sources deferred is not measured.The 13-inch M1 (
tas2770) logs the same line on both kernels. The 14-inch M2 Max stock kernel logged fourtas2764lines and the candidate none, so the deferral depends on probe timing.Checks
scripts/checkpatch.pl --stricton this patch: 0 errors, 0 warnings, 0 checks.Not tested
I did not build or boot a kernel with this patch.